Add optional internal routing #8
Conversation
| } else { | ||
| // If there are no delegated routing endpoints run an accelerated Amino DHT client and send IPNI requests to cid.contact | ||
|
|
||
| // TODO: This datastore shouldn't end up containing anything anyway so this could potentially just be a null datastore |
There was a problem hiding this comment.
I don't understand this... aren't dht records stored in this datastore?
There was a problem hiding this comment.
I mean, I thought that anything retrieved from the DHT would be stored locally too, even if you are only a client.
There was a problem hiding this comment.
We can double check, but there's no reason to implement what is effectively record caching at this layer. That's not to say the code won't do that (I suspect at least for IPNS records it will) just that it's mostly non-sensical (e.g. IPNS records caching is better done at a layer that understands IPNS records like namesys)
| fullrt.DHTOption( | ||
| dht.Validator(record.NamespacedValidator{ | ||
| "pk": record.PublicKeyValidator{}, | ||
| "ipns": ipns.Validator{KeyBook: h.Peerstore()}, | ||
| }), | ||
| dht.Datastore(memDS), | ||
| dht.BootstrapPeers(dht.GetDefaultBootstrapPeerAddrInfos()...), | ||
| dht.BucketSize(20), |
There was a problem hiding this comment.
does the fullRT dht understand dht.ModeClient? This nodes shouldn't be storing records for other people even if they are publicly reachable.
There was a problem hiding this comment.
Pretty sure it doesn't, but that's because it is only a client (i.e. it never operates in server mode)
| switch cfg.DHTType { | ||
| case Combined: | ||
| dhtRouter = &bundledDHT{ | ||
| standard: standardClient, | ||
| fullRT: fullRTClient, | ||
| } | ||
| case Standard: | ||
| dhtRouter = standardClient | ||
| case Accelerated: | ||
| dhtRouter = fullRTClient | ||
| default: | ||
| return nil, fmt.Errorf("unsupported DHT type") | ||
| } |
There was a problem hiding this comment.
I think I regular DHT should just use combined.
There was a problem hiding this comment.
Yeah, feel free to rename however you want. Just wanted to illustrate the options for you and allow us to test them out if we want to evaluate the performance of them.
| } else { | ||
| // If there are no delegated routing endpoints run an accelerated Amino DHT client and send IPNI requests to cid.contact | ||
|
|
||
| // TODO: This datastore shouldn't end up containing anything anyway so this could potentially just be a null datastore |
There was a problem hiding this comment.
I mean, I thought that anything retrieved from the DHT would be stored locally too, even if you are only a client.
|
I merge this and will finish on the original PR. Thanks!! |
Falls back to using the Amino DHT + cid.contact as an IPNI resolver if there is no routingV1 endpoint passed.
A few notes:
boxo/bitswap) would be better about how many requests they send to newly discovered non-local peers where there isn't external information that the peer is likely useful.cc @hsanjuan